-
-
Notifications
You must be signed in to change notification settings - Fork 267
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add custom validation #1236
base: master
Are you sure you want to change the base?
feat: add custom validation #1236
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1236 +/- ##
==========================================
+ Coverage 97.33% 97.55% +0.21%
==========================================
Files 42 55 +13
Lines 2104 2614 +510
==========================================
+ Hits 2048 2550 +502
- Misses 56 64 +8
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
fbf3813
to
7bd16c3
Compare
@Lee-W I was wondering if you had any input to this PR? |
I'll be mostly out till at least mid-Oct, will try to check in depth after that. Thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall, I love this idea! left a few improvement suggestions
commitizen/cz/base.py
Outdated
allow_abort: bool, | ||
allowed_prefixes: list[str], | ||
max_msg_length: int, | ||
) -> tuple[bool, list]: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we make the return type a named tuple for explicity?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added a named tuple ValidationResult
. Let me know it the solution works for you.
Description
This PR adds functionality to customise the commit message validation and to format the
InvalidCommitMessageError
to give better/more detailed feedback to the user.Checklist
./scripts/format
and./scripts/test
locally to ensure this change passes linter check and testExpected behavior
The developer of a custom commitizen class can override the
validate_commit_message
andformat_error_message
methods to perform more complex commit message format checks then just a regex match and give more detailed feedback on failure.Steps to Test This Pull Request
Run the the
test_check_command_with_custom_validator_succeed
andtest_check_command_with_custom_validator_fail
tests intest_check_command.py
.Additional context
This PR implements and fixes the comments from #648.